refactor: overwrite ArbitrageMinGasPrice config from .005 to 0.1 #7368
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
p0mvn
added
V:state/compatible/backport
p0mvn
commented
Jan 26, 2024
Comment on lines
+526
to
+543
| // Convert the content to a string | ||
| fileContent := string(content) | ||
|
|
||
| // Find the index of the search line | ||
| index := strings.Index(fileContent, arbitrageMinGasFeeConfigName) | ||
| if index == -1 { | ||
| return fmt.Errorf("search line not found in the file") | ||
| } | ||
|
|
||
| // Find the opening and closing quotes | ||
| openQuoteIndex := strings.Index(fileContent[index:], "\"") | ||
| openQuoteIndex += index | ||
|
|
||
| closingQuoteIndex := strings.Index(fileContent[openQuoteIndex+1:], "\"") | ||
| closingQuoteIndex += openQuoteIndex + 1 | ||
|
|
||
| // Replace the old value with the new value | ||
| newFileContent := fileContent[:openQuoteIndex+1] + newArbitrageMinGasFeeValue + fileContent[closingQuoteIndex:] |
There was a problem hiding this comment.
Note:
The following are the reasons why this is done by string matching manually:
- https://github.com/pelletier/go-toml does not handle comments when writing. So all config comments would be removed if I were to load the file, update the value, and overwrite.
- SDK helpers either unexpectedly overwrite other config values or run into the same comment issue as above. I found it easier to simply string-match
There was a problem hiding this comment.
I also ran into this when I attempted this a while ago, thanks for making an issue
p0mvn
changed the title
PaddyMc
approved these changes
Jan 26, 2024
czarcas7ic
reviewed
Jan 29, 2024
Comment on lines
+518
to
+519
| // Check if the file is writable | ||
| if fileInfo.Mode()&os.FileMode(0200) != 0 { |
There was a problem hiding this comment.
I think this implies if the file is not writeable, then the defer func wont trigger so there will be no feedback for the operator right?
There was a problem hiding this comment.
They would just not have their config updated which IMO is fine. The intention is to silently ignore the update if their file isn't writeable
There was a problem hiding this comment.
Right, I just think they would want to still get a log print saying it wasn't overwritten due to permission issues
mergify bot
pushed a commit
that referenced
this pull request
Jan 29, 2024
Closes: #XXX ## What is the purpose of the change To address the failing arbitrage spam taking up block space, we will increase [arbitrage-min-gas-fee](https://github.com/osmosis-labs/osmosis/blob/3aaf5d117f96df0092b0375d0c8de0a12850d092/cmd/osmosisd/cmd/root.go#L540) to be near the order of magnitude of 0.1 OSMO. CL Swap is around 600K gas wanted and 340K gas used. Let's assume a middle ground of 450K and aim to have the arb pay approximately 0.05 OSMO. ``` 0.045 = 450_000 * 0.1 / 10**6 ``` As a result, 0.1 is an appropriate choice. ## Testing and Verifying Tested the following vectors: - new config - does no overwrite other values - old value is "0.005" -> overwritten - old value is ".005" -> overwritten - old value is something else -> does not change ## Documentation and Release Note - [ ] Does this pull request introduce a new feature or user-facing behavior changes? - [ ] Changelog entry added to `Unreleased` section of `CHANGELOG.md`? Where is the change documented? - [ ] Specification (`x/{module}/README.md`) - [ ] Osmosis documentation site - [ ] Code comments? - [ ] N/A (cherry picked from commit af1ed8c)
p0mvn
added a commit
that referenced
this pull request
Feb 1, 2024
…kport #7368) (#7383) * refactor: overwrite ArbitrageMinGasPrice config from .005 to 0.1 (#7368) Closes: #XXX ## What is the purpose of the change To address the failing arbitrage spam taking up block space, we will increase [arbitrage-min-gas-fee](https://github.com/osmosis-labs/osmosis/blob/3aaf5d117f96df0092b0375d0c8de0a12850d092/cmd/osmosisd/cmd/root.go#L540) to be near the order of magnitude of 0.1 OSMO. CL Swap is around 600K gas wanted and 340K gas used. Let's assume a middle ground of 450K and aim to have the arb pay approximately 0.05 OSMO. ``` 0.045 = 450_000 * 0.1 / 10**6 ``` As a result, 0.1 is an appropriate choice. ## Testing and Verifying Tested the following vectors: - new config - does no overwrite other values - old value is "0.005" -> overwritten - old value is ".005" -> overwritten - old value is something else -> does not change ## Documentation and Release Note - [ ] Does this pull request introduce a new feature or user-facing behavior changes? - [ ] Changelog entry added to `Unreleased` section of `CHANGELOG.md`? Where is the change documented? - [ ] Specification (`x/{module}/README.md`) - [ ] Osmosis documentation site - [ ] Code comments? - [ ] N/A (cherry picked from commit af1ed8c) * changelog --------- Co-authored-by: Roman <roman@osmosis.team>
|
why not make it just 1 osmo and leave this spam ahead? |
Generally, I agree with increasing it even higher. I was worried about making too much of a rapid change for all nodes at once. This is already a 20x increase |
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #XXX
What is the purpose of the change
To address the failing arbitrage spam taking up block space, we will increase arbitrage-min-gas-fee to be near the order of magnitude of 0.1 OSMO.
CL Swap is around 600K gas wanted and 340K gas used. Let's assume a middle ground of 450K and aim to have the arb pay approximately 0.05 OSMO.
As a result, 0.1 is an appropriate choice.
Testing and Verifying
Tested the following vectors:
Documentation and Release Note
Unreleasedsection ofCHANGELOG.md?Where is the change documented?
x/{module}/README.md)